-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Implement v2-debug checks for args, config, customizations, s3, and environment variables #9775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement v2-debug checks for args, config, customizations, s3, and environment variables #9775
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## debug-migration #9775 +/- ##
==================================================
Coverage ? 93.28%
==================================================
Files ? 209
Lines ? 16885
Branches ? 0
==================================================
Hits ? 15752
Misses ? 1133
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| encountered_timestamp = False | ||
| def identity_with_warning(x): | ||
| nonlocal encountered_timestamp | ||
| if not encountered_timestamp: | ||
| encountered_timestamp = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain this flow better in comments? Is it so we're only printing a single warning per response?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, without adding these guards I was getting flooded with messages (one per timestamp in the responses), which nearly made the feature unusable.
I'll update with comments
awscli/customizations/paginate.py
Outdated
| # This function checks for pagination args in the actual calling | ||
| # arguments passed to the function. If the user is using the | ||
| # --cli-input-json parameter to provide JSON parameters they are | ||
| # all in the API naming space rather than the CLI naming space | ||
| # and would be missed by the processing above. This function gets | ||
| # called on the calling-command event. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally prefer docstrings throughout. Though we don't generate user-facing docs from these, they're more helpful in IDEs for us and contributors.
Description of changes:
--v2-debug:PYTHONUTF8andPYTHONIOENCODINGenv vars unsupported in v2.aws s3namespace may call additional APIs for multipart S3 copiesaws cloudformation deploywill not fail on empty changesets.file://with blob-type parameters.us-east-1will use the regional endpoint in v2 rather than global.api_versionsin config will not be supported in v2.[plugins]will be provisional in v2.--cli-input-jsonwill disable automatic paging in v2.--v2-debugcode.Description of tests:
PYTHONUTF8:aws s3 cp:aws s3 ls:--cli-input-json:api_versionsin config file:file://for a blob-type parameter:After setting
cli_binary_formattoraw-in-base64-out, I've verified that the warning above was not printed.us-east-1and executing an S3 command:After setting
regiontoaws-global, I've verified that the warning above was not printed.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.